-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add set layout option feature #181
base: master
Are you sure you want to change the base?
Conversation
Looks good, is this BC-compatible by default? |
} | ||
|
||
$layoutExtraAssetsOptions = self::getLayoutExtraAssetsOptions(); | ||
if (array_key_exists($layout, $layoutExtraAssetsOptions)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section could be problematic when bundling assets, since some stuff is loaded dynamically.
Might be an option to register the additional asset files manually via the ExtraAsset...
- although this is not a perfect solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this stuff (extra assets) is needed by default for specific layout. Maybe, to do it more flexible we can create AdminLteBowerAsset
that adds possibility for developers to connect scripts, styles from bower_components
directory at almasaeed2010/adminlte
package.
But somehow logic that for specific layout
options asset connect extra scripts, styles must be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its related to #180
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think also solution can be write a notes to docs that some layouts by default adds extra js, css from bower_components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But instead of setting a property, how about simply registering AdminLteFixedLayoutAsset
instead of AdminLteAsset
?
@schmunk42 by default |
According to official AdminLTE layout documentation:
layout
property atAdminLteAsset.php
\dmstr\helpers\AdminLteHelper::layoutHtmlClass()
TODO:
Related issues: